Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Resource to prepare for NSXT cluster upgrade #1069

Merged
merged 1 commit into from
Feb 23, 2024
Merged

Resource to prepare for NSXT cluster upgrade #1069

merged 1 commit into from
Feb 23, 2024

Conversation

2ez4szliu
Copy link
Contributor

@2ez4szliu 2ez4szliu commented Dec 19, 2023 •
edited
Loading

Implement resources to support the upgrade for NSXT cluster
New resources and data source include:

  1. nsxt_upgrade_prepare - upload precheck bundle and run precheck
  2. nsxt_upgrade_precheck_acknowledge - acknowledge the warnings from precheck
  3. data source nsxt_upgrade_prepare_status - Check if NSX Manager is ready for upgrade by checking if there are errors in precheck.

@2ez4szliu 2ez4szliu marked this pull request as draft December 19, 2023 03:11
@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch 2 times, most recently from c0cf7e8 to 64a22a7 Compare December 20, 2023 22:14
@salv-orlando salv-orlando added this to the v3.4.1 milestone Jan 2, 2024
@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch from 64a22a7 to 9221914 Compare January 3, 2024 01:46
@2ez4szliu 2ez4szliu changed the title [WIP] Resource to prepare for NSXT cluster upgrade Resource to prepare for NSXT cluster upgrade Jan 3, 2024
@2ez4szliu 2ez4szliu marked this pull request as ready for review January 3, 2024 01:48
@GraysonWu
Copy link
Collaborator

GraysonWu commented Jan 4, 2024 •
edited
Loading

I noticed that MP SDK is used here. Just curious if it still can work for the [latestVersion-1 ] -> [latestVersion] ? And is MP API being deprecated?

@2ez4szliu
Copy link
Contributor Author

2ez4szliu commented Jan 4, 2024 •
edited
Loading

I noticed that MP SDK is used here. Just curious if it still can work for the [latestVersion-1 ] -> [latestVersion] ? And is MP API being deprecated?

I think that at least for the APIs related cluster fabric and upgrade, they are not deprecated in MP and we still need to use them from nsxt-mp, it seems only the client function for accept user agreement are implemented in nsxt policy API, others are still in MP

@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch 2 times, most recently from 6b42c52 to f179c93 Compare January 16, 2024 22:48
@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch 12 times, most recently from c2f315a to e5df2d2 Compare January 27, 2024 06:03
@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch 2 times, most recently from cfed3fb to 0ed6d46 Compare January 29, 2024 22:35
@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch 2 times, most recently from 8e12e03 to a5606a6 Compare February 8, 2024 17:31
annakhm
annakhm reviewed Feb 8, 2024
View reviewed changes
nsxt/resource_nsxt_upgrade_prepare.go
Description: "Whether to accept the user agreement",
Required: true,
},
"bundle_upload_timeout": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to define timeouts under a sub-clause

Copy link
Contributor Author

@2ez4szliu 2ez4szliu Feb 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean that we define a schema for all timeout, whose attributes are these individual timeout values

timeout {
uc_timeout = ...
precheck_timeout = ...
}

@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch 4 times, most recently from 058fac8 to 0391ea3 Compare February 15, 2024 22:47
annakhm
annakhm reviewed Feb 16, 2024
View reviewed changes
nsxt/provider.go
@@ -317,6 +317,7 @@ func Provider() *schema.Provider {
"nsxt_edge_upgrade_group": dataSourceNsxtEdgeUpgradeGroup(),
"nsxt_host_upgrade_group": dataSourceNsxtHostUpgradeGroup(),
"nsxt_policy_gateway_interface_realization_info": dataSourceNsxtPolicyGatewayInterfaceRealizationInfo(),
"nsxt_upgrade_prepare_ready": dataSourceNsxtUpgradePrepareReady(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a naming comment - prepare_ready sounds non-optimal to me, maybe prepare_done? Other suggestions? @GraysonWu @salv-orlando

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prepare_complete? I'm ok with both

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just nsxt_upgrade_ready?

@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch 2 times, most recently from 5b5df58 to 5656b68 Compare February 17, 2024 03:36
salv-orlando
salv-orlando reviewed Feb 21, 2024
View reviewed changes
website/docs/r/upgrade_prepare.html.markdown Outdated Show resolved Hide resolved
website/docs/r/upgrade_prepare.html.markdown Outdated Show resolved Hide resolved
website/docs/r/upgrade_precheck_acknowledge.html.markdown Outdated Show resolved Hide resolved
website/docs/d/upgrade_prepare_ready.html.markdown Outdated Show resolved Hide resolved
website/docs/d/upgrade_prepare_ready.html.markdown Outdated Show resolved Hide resolved
@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch from 5656b68 to a07e7e6 Compare February 21, 2024 22:04
@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch 3 times, most recently from f53cbf4 to cbc29e0 Compare February 22, 2024 20:11
@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch 2 times, most recently from 7b0c1a3 to b086985 Compare February 23, 2024 16:51
@annakhm
Copy link
Collaborator

annakhm commented Feb 23, 2024 •
edited
Loading

When dependency on nsxt_upgrade_precheck_acknowledge resource is missing in nsxt_upgrade_prepare_ready data source, we get into never-ending cycle of prechecks and acks that never gets resolved:

  1. prepare resource initiates fresh precheck cycle that would require re-acknowledging
  2. ready data source gets evaluated right away, and acks are missing
  3. ack resource does its job, but too late

For now lets document this, but in a follow up patch I suggest to solve this with amending prepare resource to only re-run prechecks if needed, which is one of the following cases:

  1. system is ready for its first run of prechecks on this upgrade bundle (i.e, UC is upgraded etc, but we never run prechecks before)
  2. prechecks were executed before and resulted in error or warning that requires ack

we need to be careful with this change since there might be scenario when prechecks were run before, but user decided to change upgrade bundle.

@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch 2 times, most recently from 257cc08 to 8ad37c7 Compare February 23, 2024 20:05
@2ez4szliu 2ez4szliu force-pushed the nsxt-upgrade-prepare branch from 8ad37c7 to 01a346f Compare February 23, 2024 20:23
@2ez4szliu 2ez4szliu merged commit a2ad166 into vmware:master Feb 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@salv-orlando salv-orlando salv-orlando left review comments

@annakhm annakhm annakhm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.5.0
Development

Successfully merging this pull request may close these issues.
4 participants
@2ez4szliu @GraysonWu @annakhm @salv-orlando